Skip to content

x-pack/filebeat/input/cel: accept string values for secret_state#50508

Open
efd6 wants to merge 1 commit intoelastic:mainfrom
efd6:k267859-cel
Open

x-pack/filebeat/input/cel: accept string values for secret_state#50508
efd6 wants to merge 1 commit intoelastic:mainfrom
efd6:k267859-cel

Conversation

@efd6
Copy link
Copy Markdown
Contributor

@efd6 efd6 commented May 6, 2026

Proposed commit message

x-pack/filebeat/input/cel: accept string values for secret_state

Fleet resolves secrets to their stored string values, so when
secret_state is configured with secret: true in an integration
package, the agent delivers a YAML text string rather than a
parsed map. Change SecretState from map[string]interface{} to a
custom type that implements the ucfg Unpacker interface, accepting
either a map or a string and parsing the string as YAML.

This works around a Fleet bug where type: yaml variables with
secret: true are silently corrupted during policy compilation.
See https://github.com/elastic/kibana/issues/267859

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

Disruptive User Impact

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 self-assigned this May 6, 2026
@efd6 efd6 added Filebeat Filebeat bugfix Team:Security-Service Integrations Security Service Integrations Team backport-8.19 Automated backport to the 8.19 branch backport-9.3 Automated backport to the 9.3 branch backport-9.4 labels May 6, 2026
@botelastic botelastic Bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
  • /test : Run the Buildkite pipeline.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

TL;DR

The Buildkite failure is a test flake in TestConcurrentRequestsExceedHighWater: the test expected 503 but got 200 because it relies on fixed time.Sleep(150ms) timing before issuing the second request. Replace the sleep with a deterministic wait for inFlight >= high_water before sending the fast request.

Remediation

  • In x-pack/filebeat/input/http_endpoint/handler_test.go:871-885, replace time.Sleep(150 * time.Millisecond) with assert.Eventually/require.Eventually that waits until h.inFlight.Load() >= c.HighWaterInFlight, then send the fast request.
  • Re-run the specific test on Windows (go test -run TestConcurrentRequestsExceedHighWater ./x-pack/filebeat/input/http_endpoint) to confirm stability.
Investigation details

Root Cause

TestConcurrentRequestsExceedHighWater is timing-coupled:

  • It starts a slow request and then sleeps for a fixed duration (x-pack/filebeat/input/http_endpoint/handler_test.go:871-873).
  • It assumes that after that sleep, in-flight bytes are already above high water before the second request is admitted (x-pack/filebeat/input/http_endpoint/handler_test.go:875-892).

Admission control checks h.inFlight at request start (x-pack/filebeat/input/http_endpoint/handler.go:127-141). If the slow request has not yet pushed inFlight above high water on that scheduler tick, the fast request is accepted and returns 200.

Evidence

=== FAIL: x-pack/filebeat/input/http_endpoint TestConcurrentRequestsExceedHighWater (0.49s)
    handler_test.go:892:
        Error:       Not equal:
                     expected: 503
                     actual  : 200

Verification

  • Ran locally (Linux): go test -run TestConcurrentRequestsExceedHighWater -count=30 ./x-pack/filebeat/input/http_endpoint (passed in this environment).
  • The Buildkite failure is Windows-specific in this run, consistent with a scheduler/timing-sensitive test.

Follow-up

  • Consider keeping this test deterministic by synchronizing on observed inFlight state instead of wall-clock delays.

Note

🔒 Integrity filter blocked 2 items

The following items were blocked because they don't meet the GitHub integrity level.

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented May 6, 2026

Flake fixed in #50492.

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented May 6, 2026

/test

@efd6 efd6 marked this pull request as ready for review May 6, 2026 20:46
@efd6 efd6 requested a review from a team as a code owner May 6, 2026 20:46
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 0a53de75-4096-4da1-b05d-405419b8b659

📥 Commits

Reviewing files that changed from the base of the PR and between 42c97d4 and e44eb8a.

📒 Files selected for processing (5)
  • changelog/fragments/1778038687-cel-secret-state-unpack.yaml
  • x-pack/filebeat/input/cel/config.go
  • x-pack/filebeat/input/cel/config_test.go
  • x-pack/filebeat/input/cel/input.go
  • x-pack/filebeat/input/cel/input_manager.go

📝 Walkthrough

Walkthrough

Adds a new changelog fragment and changes Filebeat's CEL input to accept secret_state as either a map or a YAML string. Introduces a secretState type with an Unpack method (using gopkg.in/yaml.v3) and updates the config struct to use secretState. Code paths now reference the nested m field of secretState. A unit test TestSecretStateUnpack with five subtests was added.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@x-pack/filebeat/input/cel/config.go`:
- Around line 121-122: The YAML unmarshalling branch currently calling
yaml.Unmarshal([]byte(v), &s.m) produces map[interface{}]interface{} for nested
maps (breaking downstream type expectations); update the string case handling in
the secretState logic to normalize nested map keys to strings after
unmarshalling (or switch to gopkg.in/yaml.v3). Specifically, either replace the
yaml.Unmarshal call with yaml.v3's unmarshal into s.m, or keep yaml.v2 and add a
recursive normalization helper (e.g., normalizeMapIfaceToString) that walks the
unmarshalled value and converts map[interface{}]interface{} to
map[string]interface{} before assigning to s.m so downstream CEL/structpb code
and type assertions succeed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 768f98ae-0abc-4d6e-a731-439bdd8c5960

📥 Commits

Reviewing files that changed from the base of the PR and between 9702d0a and 42c97d4.

📒 Files selected for processing (5)
  • changelog/fragments/1778038687-cel-secret-state-unpack.yaml
  • x-pack/filebeat/input/cel/config.go
  • x-pack/filebeat/input/cel/config_test.go
  • x-pack/filebeat/input/cel/input.go
  • x-pack/filebeat/input/cel/input_manager.go

Comment thread x-pack/filebeat/input/cel/config.go
Fleet resolves secrets to their stored string values, so when
secret_state is configured with secret: true in an integration
package, the agent delivers a YAML text string rather than a
parsed map. Change SecretState from map[string]interface{} to a
custom type that implements the ucfg Unpacker interface, accepting
either a map or a string and parsing the string as YAML.

This works around a Fleet bug where type: yaml variables with
secret: true are silently corrupted during policy compilation.
See elastic/kibana#267859
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-8.19 Automated backport to the 8.19 branch backport-9.3 Automated backport to the 9.3 branch backport-9.4 bugfix Filebeat Filebeat Team:Security-Service Integrations Security Service Integrations Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant